fix(continuous-learning): bump observer MAX_TURNS default to 50 (#2035)#2057
Open
fxdv wants to merge 1 commit into
Open
fix(continuous-learning): bump observer MAX_TURNS default to 50 (#2035)#2057fxdv wants to merge 1 commit into
fxdv wants to merge 1 commit into
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR raises the observer analysis default ChangesObserver Max-Turns Default Increase
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…an-m#2035) The observer's MAX_TURNS default of 20 was systematically insufficient for the MAX_ANALYSIS_LINES default of 500. First-cycle analysis on a fresh project consistently failed with "Reached max turns (20)", forcing users to either raise ECC_OBSERVER_MAX_TURNS or lower ECC_OBSERVER_MAX_ANALYSIS_LINES before the observer became useful. Pair the defaults so the out-of-the-box experience succeeds: bump MAX_TURNS to 50 (the value the reporter empirically settled on for the 500-line default). The safety floor (turns < 4 falls back to default) is preserved. Test asserting the default constant is updated alongside the source.
fxdv
force-pushed
the
fix/observer-max-turns-2035
branch
from
6a2645a to
83b6048
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2035. The observer's
MAX_TURNSdefault of20was systematically too small for theMAX_ANALYSIS_LINESdefault of500. First-cycle analysis on a fresh project consistently failed withReached max turns (20), forcing users to either raiseECC_OBSERVER_MAX_TURNSor lowerECC_OBSERVER_MAX_ANALYSIS_LINESbefore the observer became useful.This PR pairs the defaults so the out-of-the-box experience succeeds:
MAX_TURNSis bumped to50— the value the reporter empirically settled on for the 500-line default.Changes
skills/continuous-learning-v2/agents/observer-loop.sh: change three default-site20s to50(lines 208, 213, 218) and add a comment explaining why the defaults must stay paired.tests/hooks/hooks.test.js: update the assertion that pins the default constant so it tracks the new value.The existing safety floor (turns
< 4falls back to default) is preserved.Test plan
node tests/hooks/hooks.test.js— 237/237 pass (including the updatedobserver-loop uses a configurable max-turn budget with safe defaulttest)ECC_OBSERVER_MAX_TURNS=10still respected) and non-numeric / sub-4 fallback paths still resolve to the new50defaultECC_OBSERVER_MAX_TURNSormax_turns=20exist in the repo (rgconfirmed)Notes for reviewer
I considered an auto-scale approach (e.g.
max_turns = ceil(max_lines / 10)) but chose the conservative constant change to minimize blast radius and match the reporter's suggested fix shape. Happy to fold an auto-scale variant in if preferred.Summary by cubic
Bumps the observer’s default
MAX_TURNSfrom 20 to 50 to match the 500-line analysis default, preventing first-cycle “Reached max turns” failures (addresses #2035).ECC_OBSERVER_MAX_TURNSoverrides still work, and the <4 safety fallback stays.skills/continuous-learning-v2/agents/observer-loop.shto defaultmax_turnsto 50 and adjust fallbacks; add a comment linking the default toMAX_ANALYSIS_LINES=500.tests/hooks/hooks.test.jsto assert the new 50-turn default.Written for commit 6a2645a. Summary will update on new commits. Review in cubic
Summary by CodeRabbit